Use ts-jest instead of custom solution for transforming ts files #100
Conversation
| @@ -36,7 +36,7 @@ module.exports = (resolve, rootDir) => { | |||
| testURL: 'http://localhost', | |||
| transform: { | |||
| '^.+\\.css$': resolve('config/jest/cssTransform.js'), | |||
| '^.+\\.tsx?$': resolve('config/jest/typescriptTransform.js'), | |||
| '^.+\\.tsx?$': resolve('node_modules/ts-jest/preprocessor.js'), | |||
There was a problem hiding this comment.
This will be node_modules/react-scripts-ts/ts-jest/preprocessor.js - at least until the user has ejected. Which I think we'll need to consider?
There was a problem hiding this comment.
I'm not sure what exactly should be used here.
My first attempt was to use
resolve('<rootDir>/node_modules/ts-jest/preprocessor.js')
but that failed on the first run - figure out that resolve does a simple path joining here. Using the line above the first test part (L127 in e2e-simple.sh) passes, but the second one (with the generated test project, L264) fails.
In an ejected project, this should be
'^.+\\.tsx?$': '<rootDir>/node_modules/ts-jest/preprocessor.js'
Using this directly also fails on the first test run, since <rootDir> references a different project root not containing ts-jest.
It seems that it'd be required to use different paths in both situations, but I'm still trying to figure out how to set this up properly.
There was a problem hiding this comment.
Now using the former file as a require-wrapper to work around the problems with different root paths.
| @@ -45,6 +45,9 @@ module.exports = (resolve, rootDir) => { | |||
| moduleNameMapper: { | |||
| '^react-native$': 'react-native-web', | |||
| }, | |||
| globals: { | |||
| __TS_CONFIG__: resolve('template/tsconfig.json'), | |||
There was a problem hiding this comment.
This won't be inside template once an app has been initialised. This was used previously:
const tsconfigPath = require('app-root-path').resolve('/tsconfig.json');
which worked well!
There was a problem hiding this comment.
I've tried that already, but again it's problematic in terms of the project root in use during the tests.
When running the first test (L127 of e2e-simple.sh), tsconfigPath determined the way you mentioned above would reference
create-react-app-typescript/packages/react-scripts/tsconfig.json
since the react-scripts folder is the current directory when running this test. Apparently, this file does not exist - the tsconfig.json is only present in the template.
Again, it might be required to use different paths here. Trying to figure out how. This __TS_CONFIG__ entry is only required in case the tsconfig.json is not present in the current root - same behavior as with the tsc cli.
€dit: We might go with a conditional path entry in paths.js here -
In case it's not linked:
appTsConfig: resolveApp('tsconfig.json')
Otherwise:
appTsConfig: resolveOwn('template/tsconfig.json')
So the entry above would become
globals: {
__TS_CONFIG__: paths.appTsConfig,
}
There was a problem hiding this comment.
This should be done - amended.
|
Thanks for this @DorianGrey! I will wait to see what @zinserjan thinks, as as you said he worked on a bunch of stuff to get this fixed up. |
|
Thanks for letting me now. I'll look into it after work. |
|
@DorianGrey I tried this out on my fork and made some changes to get this (partially) working for me (see zinserjan@d1fd158). With partially I mean that I have some issues when I use Seems that ts-jest does not support that. To get my tests running I hacked a bit in ts-jest and removed the prepending of the sourcemap install. Another issue could be the usage of node-source-map-support which impacts the performance as it transpiles all referenced ts files for a stacktrace instead of using the cached version. But I think the impact is quite slight, cause this should concern only failed tests. Anyway I think this is the right way to go and makes the maintenance of this CRA fork hopefully easier. |
|
Thanks for the review @zinserjan. Yet, I'm still scratching my head about the second and third e2e test (L264 of And the test still fails when executed locally. I've executed the step mentioned manually. It seems to generate a project with an outdated |
|
@DorianGrey I think the reason it has issues when running locally is to do with a cached version is builds for the version. And as it doesn't change it doesn't work. As this is all passing on CI I assume it's fine to merge? |
|
Just rebased everything. |
Hi there,
as already proposed in #99, I'd suggest it'd favorable to use an already existing and maintained preprocessor for typescript files, instead of using a custom solution that attempts to not only do the same, but in the same way as well.
I'm proposing ts-jest for this purpose - already using it in testing and production projects, and it works fine. This PR's intent is to discuss and figure out if ts-jest is suitable for the requirements of this project, or if it'd too much or has some issues that still would need to be fixed before it can be taken into consideration.
Besides, I hope I've found all spots that require to be changed for this adoption...
Replacing the current solution with ts-jest would also replace the work that has already been done on the current typescript transformer. Namely, it would affect the PRs:
Effect: Superseed
Effect: Replace
@zinserjan, as some of these have been provided by you, it'd good if you can a second look for the current ts-jest implementation to match the changes from these PRs in an acceptable way. Esp. the function to create the cache key (see here) should be of interest.